Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix node InternalIP not matching host-only address #5427

Merged
merged 1 commit into from
Sep 23, 2019
Merged

Fix node InternalIP not matching host-only address #5427

merged 1 commit into from
Sep 23, 2019

Conversation

woodcockjosh
Copy link
Contributor

This fixes the issue with the node InternalIP not matching the minikube ip

@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Sep 21, 2019
@k8s-ci-robot
Copy link
Contributor

Hi @woodcockjosh. Thanks for your PR.

I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: woodcockjosh
To complete the pull request process, please assign sharifelgamal
You can assign the PR to them by writing /assign @sharifelgamal in a comment when ready.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Sep 21, 2019
@minikube-bot
Copy link
Collaborator

Can one of the admins verify this patch?

@TravisBuddy
Copy link

Travis tests have failed

Hey @woodcockjosh,
Please read the following log in order to understand the failure reason.
It'll be awesome if you fix what's wrong and commit the changes.

1st Build

View build log

make test 
which go-bindata || GO111MODULE=off GOBIN=/home/travis/gopath/bin go get github.com/jteeuwen/go-bindata/...
PATH="/home/travis/gopath/bin:/home/travis/.gimme/versions/go1.12.9.linux.amd64/bin:/home/travis/bin:/home/travis/bin:/home/travis/.local/bin:/usr/local/lib/jvm/openjdk11/bin:/opt/pyenv/shims:/home/travis/.phpenv/shims:/home/travis/perl5/perlbrew/bin:/home/travis/.nvm/versions/node/v8.12.0/bin:/home/travis/.rvm/gems/ruby-2.5.3/bin:/home/travis/.rvm/gems/ruby-2.5.3@global/bin:/home/travis/.rvm/rubies/ruby-2.5.3/bin:/home/travis/gopath/bin:/home/travis/.gimme/versions/go1.11.1.linux.amd64/bin:/usr/local/maven-3.6.0/bin:/usr/local/cmake-3.12.4/bin:/usr/local/clang-7.0.0/bin:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin:/snap/bin:/home/travis/.rvm/bin:/home/travis/.phpenv/bin:/opt/pyenv/bin:/home/travis/.yarn/bin:/home/travis/gopath/bin" go-bindata -nomemcopy -o pkg/minikube/assets/assets.go -pkg assets deploy/addons/...
gofmt -s -w pkg/minikube/assets/assets.go
which go-bindata || GO111MODULE=off GOBIN=/home/travis/gopath/bin go get github.com/jteeuwen/go-bindata/...
/home/travis/gopath/bin/go-bindata
PATH="/home/travis/gopath/bin:/home/travis/.gimme/versions/go1.12.9.linux.amd64/bin:/home/travis/bin:/home/travis/bin:/home/travis/.local/bin:/usr/local/lib/jvm/openjdk11/bin:/opt/pyenv/shims:/home/travis/.phpenv/shims:/home/travis/perl5/perlbrew/bin:/home/travis/.nvm/versions/node/v8.12.0/bin:/home/travis/.rvm/gems/ruby-2.5.3/bin:/home/travis/.rvm/gems/ruby-2.5.3@global/bin:/home/travis/.rvm/rubies/ruby-2.5.3/bin:/home/travis/gopath/bin:/home/travis/.gimme/versions/go1.11.1.linux.amd64/bin:/usr/local/maven-3.6.0/bin:/usr/local/cmake-3.12.4/bin:/usr/local/clang-7.0.0/bin:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin:/snap/bin:/home/travis/.rvm/bin:/home/travis/.phpenv/bin:/opt/pyenv/bin:/home/travis/.yarn/bin:/home/travis/gopath/bin" go-bindata -nomemcopy -o pkg/minikube/translate/translations.go -pkg translate translations/...
gofmt -s -w pkg/minikube/translate/translations.go
./test.sh
= go mod ================================================================
ok
= make lint =============================================================
golangci/golangci-lint info checking GitHub for tag 'v1.18.0'
golangci/golangci-lint info found version: 1.18.0 for v1.18.0/linux/amd64
golangci/golangci-lint info installed out/linters/golangci-lint
ok
= boilerplate ===========================================================
ok
= schema_check ==========================================================
ok
= go test ===============================================================
ok  	k8s.io/minikube/cmd/minikube/cmd	0.068s	coverage: 16.0% of statements
ok  	k8s.io/minikube/cmd/minikube/cmd/config	0.051s	coverage: 18.6% of statements
ok  	k8s.io/minikube/pkg/drivers	0.024s	coverage: 13.4% of statements
ok  	k8s.io/minikube/pkg/drivers/kvm	0.063s	coverage: 2.3% of statements
ok  	k8s.io/minikube/pkg/minikube/assets	0.031s	coverage: 64.4% of statements
ok  	k8s.io/minikube/pkg/minikube/bootstrapper	3.236s	coverage: 73.3% of statements
--- FAIL: TestGenerateKubeletConfig (0.00s)
    --- FAIL: TestGenerateKubeletConfig/old_docker (0.00s)
        kubeadm_test.go:141: unexpected diff:
            --- Expected
            +++ Got
            @@ -5,3 +5,3 @@
             ExecStart=
            -ExecStart=/var/lib/minikube/binaries/v1.11.10/kubelet --allow-privileged=true --authorization-mode=Webhook --bootstrap-kubeconfig=/etc/kubernetes/bootstrap-kubelet.conf --cadvisor-port=0 --cgroup-driver=cgroupfs --client-ca-file=/var/lib/minikube/certs/ca.crt --cluster-dns=10.96.0.10 --cluster-domain=cluster.local --container-runtime=docker --fail-swap-on=false --hostname-override=minikube --kubeconfig=/etc/kubernetes/kubelet.conf --pod-manifest-path=/etc/kubernetes/manifests
            +ExecStart=/var/lib/minikube/binaries/v1.11.10/kubelet --allow-privileged=true --authorization-mode=Webhook --bootstrap-kubeconfig=/etc/kubernetes/bootstrap-kubelet.conf --cadvisor-port=0 --cgroup-driver=cgroupfs --client-ca-file=/var/lib/minikube/certs/ca.crt --cluster-dns=10.96.0.10 --cluster-domain=cluster.local --container-runtime=docker --fail-swap-on=false --hostname-override=minikube --kubeconfig=/etc/kubernetes/kubelet.conf --node-ip=192.168.1.100 --pod-manifest-path=/etc/kubernetes/manifests
             
    --- FAIL: TestGenerateKubeletConfig/newest_cri_runtime (0.00s)
        kubeadm_test.go:141: unexpected diff:
            --- Expected
            +++ Got
            @@ -5,3 +5,3 @@
             ExecStart=
            -ExecStart=/var/lib/minikube/binaries/v1.16.0/kubelet --authorization-mode=Webhook --bootstrap-kubeconfig=/etc/kubernetes/bootstrap-kubelet.conf --cgroup-driver=cgroupfs --client-ca-file=/var/lib/minikube/certs/ca.crt --cluster-dns=10.96.0.10 --cluster-domain=cluster.local --container-runtime=remote --container-runtime-endpoint=/var/run/crio/crio.sock --fail-swap-on=false --hostname-override=minikube --image-service-endpoint=/var/run/crio/crio.sock --kubeconfig=/etc/kubernetes/kubelet.conf --pod-manifest-path=/etc/kubernetes/manifests --runtime-request-timeout=15m
            +ExecStart=/var/lib/minikube/binaries/v1.16.0/kubelet --authorization-mode=Webhook --bootstrap-kubeconfig=/etc/kubernetes/bootstrap-kubelet.conf --cgroup-driver=cgroupfs --client-ca-file=/var/lib/minikube/certs/ca.crt --cluster-dns=10.96.0.10 --cluster-domain=cluster.local --container-runtime=remote --container-runtime-endpoint=/var/run/crio/crio.sock --fail-swap-on=false --hostname-override=minikube --image-service-endpoint=/var/run/crio/crio.sock --kubeconfig=/etc/kubernetes/kubelet.conf --node-ip=192.168.1.100 --pod-manifest-path=/etc/kubernetes/manifests --runtime-request-timeout=15m
             
    --- FAIL: TestGenerateKubeletConfig/default_containerd_runtime (0.00s)
        kubeadm_test.go:141: unexpected diff:
            --- Expected
            +++ Got
            @@ -5,3 +5,3 @@
             ExecStart=
            -ExecStart=/var/lib/minikube/binaries/v1.16.0/kubelet --authorization-mode=Webhook --bootstrap-kubeconfig=/etc/kubernetes/bootstrap-kubelet.conf --cgroup-driver=cgroupfs --client-ca-file=/var/lib/minikube/certs/ca.crt --cluster-dns=10.96.0.10 --cluster-domain=cluster.local --container-runtime=remote --container-runtime-endpoint=unix:///run/containerd/containerd.sock --fail-swap-on=false --hostname-override=minikube --image-service-endpoint=unix:///run/containerd/containerd.sock --kubeconfig=/etc/kubernetes/kubelet.conf --pod-manifest-path=/etc/kubernetes/manifests --runtime-request-timeout=15m
            +ExecStart=/var/lib/minikube/binaries/v1.16.0/kubelet --authorization-mode=Webhook --bootstrap-kubeconfig=/etc/kubernetes/bootstrap-kubelet.conf --cgroup-driver=cgroupfs --client-ca-file=/var/lib/minikube/certs/ca.crt --cluster-dns=10.96.0.10 --cluster-domain=cluster.local --container-runtime=remote --container-runtime-endpoint=unix:///run/containerd/containerd.sock --fail-swap-on=false --hostname-override=minikube --image-service-endpoint=unix:///run/containerd/containerd.sock --kubeconfig=/etc/kubernetes/kubelet.conf --node-ip=192.168.1.100 --pod-manifest-path=/etc/kubernetes/manifests --runtime-request-timeout=15m
             
    --- FAIL: TestGenerateKubeletConfig/docker_with_custom_image_repository (0.00s)
        kubeadm_test.go:141: unexpected diff:
            --- Expected
            +++ Got
            @@ -5,3 +5,3 @@
             ExecStart=
            -ExecStart=/var/lib/minikube/binaries/v1.16.0/kubelet --authorization-mode=Webhook --bootstrap-kubeconfig=/etc/kubernetes/bootstrap-kubelet.conf --cgroup-driver=cgroupfs --client-ca-file=/var/lib/minikube/certs/ca.crt --cluster-dns=10.96.0.10 --cluster-domain=cluster.local --container-runtime=docker --fail-swap-on=false --hostname-override=minikube --kubeconfig=/etc/kubernetes/kubelet.conf --pod-infra-container-image=docker-proxy-image.io/google_containers/pause:3.1 --pod-manifest-path=/etc/kubernetes/manifests
            +ExecStart=/var/lib/minikube/binaries/v1.16.0/kubelet --authorization-mode=Webhook --bootstrap-kubeconfig=/etc/kubernetes/bootstrap-kubelet.conf --cgroup-driver=cgroupfs --client-ca-file=/var/lib/minikube/certs/ca.crt --cluster-dns=10.96.0.10 --cluster-domain=cluster.local --container-runtime=docker --fail-swap-on=false --hostname-override=minikube --kubeconfig=/etc/kubernetes/kubelet.conf --node-ip=192.168.1.100 --pod-infra-container-image=docker-proxy-image.io/google_containers/pause:3.1 --pod-manifest-path=/etc/kubernetes/manifests
             
FAIL
coverage: 30.9% of statements
FAIL	k8s.io/minikube/pkg/minikube/bootstrapper/kubeadm	0.058s
ok  	k8s.io/minikube/pkg/minikube/cluster	0.336s	coverage: 53.7% of statements
ok  	k8s.io/minikube/pkg/minikube/config	0.019s	coverage: 76.0% of statements
ok  	k8s.io/minikube/pkg/minikube/cruntime	0.017s	coverage: 62.4% of statements
ok  	k8s.io/minikube/pkg/minikube/extract	0.012s	coverage: 56.7% of statements
ok  	k8s.io/minikube/pkg/minikube/kubeconfig	0.042s	coverage: 75.9% of statements
ok  	k8s.io/minikube/pkg/minikube/logs	0.026s	coverage: 1.4% of statements
ok  	k8s.io/minikube/pkg/minikube/machine	0.033s	coverage: 11.2% of statements
ok  	k8s.io/minikube/pkg/minikube/notify	0.051s	coverage: 78.6% of statements
ok  	k8s.io/minikube/pkg/minikube/out	0.009s	coverage: 70.3% of statements
ok  	k8s.io/minikube/pkg/minikube/problem	0.012s	coverage: 100.0% of statements
ok  	k8s.io/minikube/pkg/minikube/proxy	0.011s	coverage: 100.0% of statements
ok  	k8s.io/minikube/pkg/minikube/registry	0.050s	coverage: 100.0% of statements
ok  	k8s.io/minikube/pkg/minikube/service	1.563s	coverage: 85.2% of statements
ok  	k8s.io/minikube/pkg/minikube/sshutil	0.561s	coverage: 100.0% of statements
ok  	k8s.io/minikube/pkg/minikube/storageclass	0.020s	coverage: 97.2% of statements
ok  	k8s.io/minikube/pkg/minikube/translate	0.011s	coverage: 8.4% of statements
ok  	k8s.io/minikube/pkg/minikube/tunnel	2.076s	coverage: 64.6% of statements
ok  	k8s.io/minikube/pkg/util	0.864s	coverage: 62.0% of statements
ok  	k8s.io/minikube/pkg/util/lock	0.010s	coverage: 59.1% of statements
ok  	k8s.io/minikube/pkg/util/retry	0.002s	coverage: 0.0% of statements
Makefile:232: recipe for target 'test' failed
make: *** [test] Error 16
TravisBuddy Request Identifier: f41b5c70-dca1-11e9-8378-0b412c1187f6

@k8s-ci-robot k8s-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Sep 21, 2019
@woodcockjosh
Copy link
Contributor Author

#3416

@afbjorklund
Copy link
Collaborator

Pretty sure that the config was kubelet.node-ip (rather than just "node-ip"), but we will see

/ok-to-test

@k8s-ci-robot k8s-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Sep 22, 2019
@woodcockjosh
Copy link
Contributor Author

woodcockjosh commented Sep 22, 2019 •
edited
Loading

Pretty sure that the config was kubelet.node-ip (rather than just "node-ip"), but we will see

/ok-to-test

yeah it is inside the NewKubeletConfig function which filters the k8s.ExtraOptions on line 540. The kubelet part of the kubelet.node-ip option just sets the context to the kubelet config. The actual kubeadm option is --node-ip https://kubernetes.io/docs/reference/command-line-tools-reference/kubelet/

@afbjorklund
Copy link
Collaborator

Ok, makes sense.

@woodcockjosh
Copy link
Contributor Author

/retest

@tstromberg
Copy link
Contributor

This is a perfect example where a long-standing bug has a simple solution, as the true difficulty is in discovering where it went wrong.

Well done, and Thank You!

@tstromberg tstromberg merged commit fb34a99 into kubernetes:master Sep 23, 2019
@medyagh medyagh mentioned this pull request Dec 16, 2019
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sharifelgamal sharifelgamal Awaiting requested review from sharifelgamal

@tstromberg tstromberg Awaiting requested review from tstromberg

Assignees
No one assigned
Labels
cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. ok-to-test Indicates a non-member PR verified by an org member that is safe to test. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@woodcockjosh @k8s-ci-robot @minikube-bot @TravisBuddy @afbjorklund @tstromberg